Skip to content

Flatten BoundKind into BoundExpression - #9256

Merged
joseph-isaacs merged 2 commits into
developfrom
mk/bound-expression-enum
Aug 13, 2026
Merged

Flatten BoundKind into BoundExpression#9256
joseph-isaacs merged 2 commits into
developfrom
mk/bound-expression-enum

Conversation

@mhk197

@mhk197 mhk197 commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Collapses BoundExpression { kind: BoundKind, dtype } into a single enum, with dtype on each variant.

pub enum BoundExpression {
    Scalar { dtype: DType, scalar_fn: ScalarFnRef, children: Arc<Vec<BoundExpression>> },
    Root { dtype: DType },
}

This is a prerequisite for lambdas, which have no dtype but should be a variant.

@mhk197 mhk197 added the changelog/skip Do not list PR in the changelog label Aug 7, 2026
@mhk197 mhk197 changed the title flatten BoundKind into BoundExpression Flatten BoundKind into BoundExpression Aug 7, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 1 regressed benchmark
✅ 1991 untouched benchmarks
⏩ 89 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation compress_fsst[(1000, 64, 8)] 1 ms 1.1 ms -10.56%
⚡ Simulation take[core/shuffled/primitive/nonnull/chunks=1024/indices=10000] 15.3 ms 13.4 ms +14.24%
⚡ WallTime words_gather_scalar[65536] 9.4 µs 8.2 µs +13.81%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

Comparing mk/bound-expression-enum (19877cb) with develop (204d1d4)

Open in CodSpeed

Footnotes

  1. 89 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@mhk197 mhk197 mentioned this pull request Aug 7, 2026
Open
@mhk197
mhk197 force-pushed the mk/bound-expression-enum branch from 93a164f to 9296dac Compare August 7, 2026 03:06
@mhk197 mhk197 added changelog/chore A trivial change and removed changelog/skip Do not list PR in the changelog labels Aug 7, 2026
@mhk197
mhk197 marked this pull request as ready for review August 7, 2026 03:21
@mhk197
mhk197 force-pushed the mk/bound-expression-enum branch from 9296dac to 6c2ee4a Compare August 11, 2026 16:24
Base automatically changed from mk/root-expression to develop August 11, 2026 16:44
@mhk197
mhk197 force-pushed the mk/bound-expression-enum branch from a664a73 to 68f9557 Compare August 11, 2026 16:45
mhk197 added 2 commits August 12, 2026 09:51
@mhk197
flatten BoundKind into BoundExpression
a9d023b 
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
fix
19877cb 
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
mhk197 force-pushed the mk/bound-expression-enum branch from 68f9557 to 19877cb Compare August 12, 2026 16:51
joseph-isaacs
joseph-isaacs reviewed Aug 13, 2026
View reviewed changes
Comment thread vortex-array/src/expr/bound_expression.rs
(
BoundExpression::Root { dtype: lhs_dtype },
BoundExpression::Root { dtype: rhs_dtype },
) => lhs_dtype == rhs_dtype,

@joseph-isaacs joseph-isaacs Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slow to check!

@joseph-isaacs joseph-isaacs Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it was already......

@joseph-isaacs
joseph-isaacs merged commit f8d4db9 into develop Aug 13, 2026
127 of 153 checks passed
@joseph-isaacs
joseph-isaacs deleted the mk/bound-expression-enum branch August 13, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joseph-isaacs joseph-isaacs joseph-isaacs approved these changes

Assignees

No one assigned

Labels

changelog/chore A trivial change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mhk197 @joseph-isaacs